-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Hotfix/astro 1458 monitor icon fix #134
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good
@@ -84,7 +84,12 @@ export class RuxMonitoringProgressIcon extends RuxMonitoringIcon { | |||
|
|||
get iconTemplate() { | |||
return html` | |||
<rux-icon icon="progress" class="rux-status--${this.status}"></rux-icon> | |||
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 128 128" style="" class="rux-status--${this.status}"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are you resorting to using the svg instad of the rux-icon component ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is because we need a variable stroke length for the SVG depending on progress %.
Passing all of these svg attributes down through the icon component for this single edge case doesn't make sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5.1 removed the progress icon entirely. Because it's a one off, I just inlined the svg.
Fixes
ASTRO-1458
New bug I just discovered where the progress was rendering 100% if the threshold was set to 100.
Breaking Change:
Icon size was changed to 44px, instead of 43.19. This is to fix a consistency issue involving rux-icon. The readme for rux-icon lists: 'supported values are extra-small (16px), small (32px), normal (44px) and large (64px)'.
Description:
5.1 removed the progress icon entirely. Because it's a one off, I just inlined the svg. Because I'm no longer using the rux-icon component, the colors weren't working so shortest, fastest solution was to just copy them back in from rux-monitoring-icon scoped under the new svg element.
We can use the stencil rewrite to refactor a lot of this. This fixes the issue and provides with a base for our regression tests.